Skip to content

Conversation

@lowBudgetHypothermia
Copy link
Collaborator

The TSM-buffer size was increased to 16 MiB. As a consquence of this the TSM buffer has to be allocated on the heap for ltsmc.c and common.c to prevent segmentation faults.

The TSM-buffer size was increased to 16 MiB. As a consquence of
this the TSM buffer has to be allocated on the heap for ltsmc.c
and common.c to prevent segmentation faults.
Copy link
Member

@ChristianTackeGSI ChristianTackeGSI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good!

Copy link
Member

@ChristianTackeGSI ChristianTackeGSI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good for now. :-)

I assume this was tested somewhat?

Comment on lines +184 to +190
int rc_minor;

rc_minor = fclose(file);
if (rc_minor) {
rc_minor = -errno;
CT_ERROR(rc_minor, "fclose failed on '%s'", filename);
if (buf) free(buf);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe move the if (buf) free(buf); above the int rc_minor;?

Something like this?

if (buf) free(buf);
buf = NULL;

int rc_minor = fclose(file);

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Concerns were raised in private that this could clobber some return values / errno. I appreciate the thoughtfulness about errno preservation in general.

Well.

  1. The free() function returns no value, and preserves errno.

    (from "man 3 free" on my Linux System)

    If anyone has a reference to an implementation or documentation indicating that free() can modify errno, please share it.

  2. The free() is just below the while loop.

    • What errno should it clobber there? By feof()? Well according to my manpages, it does not set errno either.
    • The fclose() (where we want to preserve errno!) is just below the free().

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants